Code Review

Summary: This is a correct, targeted fix for the ImplicitUsings-off compilation failure (CS1061). The root cause is sound, the approach works, and the snapshots are properly updated.

A few observations worth considering:

1. Alternative: Array.ConvertAll instead of Enumerable.Select + Enumerable.ToArray

The generated code converts a segment of args[] into a typed array. System.Array.ConvertAll is designed exactly for this:

// Current (fully-qualified LINQ, nested calls):
global::System.Linq.Enumerable.ToArray(
    global::System.Linq.Enumerable.Select(
        global::System.Linq.Enumerable.Range(0, args.Length),
        i => global::TUnit.Core.Helpers.CastHelper.Cast<T>(args[i])))

// Alternative (no LINQ dependency at all):
global::System.Array.ConvertAll(
    global::System.Array.Copy / subarray approach...)

Because args is object[], you can't directly pass it to Array.ConvertAll<object, T> without a sub-slice. For the start == 0 case it would be straightforward. For the offset case (Range(1, args.Length - 1)) you'd still need a sub-array or an index-arithmetic lambda.

A simpler LINQ-free alternative that avoids creating an intermediate IEnumerable<int> via Range:

// In TupleArgumentHelper.cs, emit:
var result = new T[args.Length - start];
for (int i = 0; i < result.Length; i++) result[i] = CastHelper.Cast<T>(args[start + i]);

This is more allocation-efficient than Range + Select + ToArray (three allocations vs. one), and requires no LINQ at all. Since this runs at test discovery time rather than a hot path it is not critical, but the Performance First principle from CLAUDE.md suggests it is worth noting. The fix as-written is acceptable though.

2. Minor: The DynamicTestsGenerator still emits using global::System.Linq;

In DynamicTestsGenerator.cs line 92, the generated file still adds using global::System.Linq;. That's fine for its own generated code (it presumably uses LINQ elsewhere in those files), but it means the TupleArgumentHelper-generated binding code that ends up in those files would not have had the CS1061 bug anyway. The bug only affected the test-registration generated files (the .g.cs files from the main test source generator), not the dynamic-tests path.

The fix correctly targets TupleArgumentHelper.cs which feeds the test-registration path. Just noting this so the scope is clear.

3. The test harness blind-spot acknowledgement is good

The PR description correctly identifies that TestsBase.cs injects global using global::System.Linq;, masking this class of bug. The regenerated snapshots now lock the fully-qualified form. Consider adding a code comment in TestsBase.cs near that global using injection warning that it intentionally hides missing-using issues:

// WARNING: This global using means snapshot tests will NOT catch CS1061-style
// missing-using bugs in generated code. Generated code must use fully-qualified
// static calls (e.g. Enumerable.ToArray) rather than extension methods.

This would make the constraint explicit for future contributors adding new LINQ calls to generated output.

Verdict

The fix is correct and complete. The approach (fully-qualifying the static method calls) is the standard idiom for source generators and matches what this codebase already does elsewhere (e.g. MetadataGenerationHelper.cs uses global::System.Linq.Enumerable.FirstOrDefault). The snapshots are properly updated. The note about Array.ConvertAll is a suggestion for a follow-up micro-optimization, not a blocker.

Code Review (Updated)

Following up on the previous review — one real bug was found that the earlier pass missed.

Bug: Regression test never exercises the dynamic LINQ code path

File: TUnit.Core.SourceGenerator.Tests/ParamsArrayCompilationTests.cs, lines 267–298

The new test Generated_params_binding_compiles_without_System_Linq_in_scope is intended to verify that the generated params-array binding compiles without using System.Linq. However, the test cases provided never reach the default arm where the LINQ calls (Enumerable.ToArray(Enumerable.Select(Enumerable.Range(...), ...))) live.

Why: The generator creates static switch arms for args.Length == 0 through args.Length == 10 (cap is maxParamsCases = 10). Only the default: arm uses the dynamic LINQ path. The test provides:

• [Arguments(RollingOrder.NewestFirst, 2, 1)] → 3 total args → hits case 3 → uses new int[] { ... } (no LINQ)
• [Arguments("a", "b", "c")] → 3 args → hits case 3 → uses new string[] { ... } (no LINQ)

The static case arms use new T[] construction with no LINQ at all. If this fix were reverted so the default: arm emitted .Select(...).ToArray() extension methods, this test would still pass because it never reaches that arm.

How to fix: Add a test invocation with more than 10 arguments to force the default arm:

// In Source string — add a method with 11+ [Arguments] values:
[Test]
[Arguments(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11)]
public void ManyParams(params int[] values)
{
}

This forces args.Length = 11 > 10 = maxParamsCases, which hits the default: arm and exercises the Enumerable.ToArray(Enumerable.Select(Enumerable.Range(...))) code that is the actual fix being guarded.

Previous findings (still relevant)

The suggestions from the prior review remain open:

1. Array.ConvertAll / loop alternative — no LINQ dependency at all, one fewer allocation. Not a blocker but worth a follow-up.
2. TestsBase.cs warning comment — the injected global using System.Linq continues to mask this class of bug for all snapshot tests. A comment there would help future contributors.

Core fix verdict

The change to TupleArgumentHelper.cs itself is correct: the static Enumerable.ToArray(Enumerable.Select(Enumerable.Range(...))) form is the standard source-generator idiom, argument ordering is right, the rangeCount arithmetic is correct, and the absolute-index lambda is preserved. The snapshots are properly updated.

The one actionable item is the test coverage gap — the regression guard won't catch a revert to extension-method form without the fix above.

Code Review

Overall: Clean, correct fix for a real bug. The root-cause diagnosis is accurate and the test strategy is well-thought-out. A few notes below.

Core Fix —

The one-line change is correct. Extension methods require using System.Linq in scope; generated .g.cs files only have that when ImplicitUsings is enabled. Switching to fully-qualified static invocation is exactly right.

Architectural suggestion: Consider eliminating LINQ from generated output entirely rather than qualifying it. The pattern is a simple index-based cast loop — emitting a for loop or a new T[count] + indexed init avoids any dependency on System.Linq in generated code and would be more explicit about what's happening:

// Instead of the current 3-level nested Enumerable call:
// var array = new T[count]; for (int i = 0; i < count; i++) array[i] = Cast<T>(args[start + i]);

This approach:

• Is easier to read in the generated output (important for debugging)
• Removes the Enumerable dependency entirely rather than working around its surface-level form
• Avoids IEnumerable intermediate allocation (though this is minor since ToArray follows immediately)

The current fix is correct for the issue at hand — this is a suggestion for a follow-up if you want to clean up generated code ergonomics.

New Test —

This is the right kind of regression test. It specifically targets the blind spot you identified in TestsBase.cs: snapshot tests compile with injected global usings, so they can't catch missing-using bugs. Compiling the generated output separately without System.Linq in scope correctly isolates the regression.

One note: output.GetDiagnostics() on the updated Compilation runs semantic analysis across all syntax trees including generated ones, so CS1061 will surface here — the approach is sound.

Minor: the generated variable captured for failure output is a good debugging touch.

Comment

The warning explaining why snapshot tests won't catch this class of bug is a valuable addition. Future contributors will understand why ParamsArrayCompilationTests exists and why generated code must use fully-qualified calls.

Minor Notes

• The new string[0] / new int[0] fallback in generated code could use Array.Empty<string>() / Array.Empty<int>() to avoid per-call allocations, but this is cosmetic.
• The BOM removal (// <auto-generated/> → // <auto-generated/>) in several snapshot files is a harmless bonus cleanup.

Verdict: The fix is correct and the test coverage addresses the specific failure mode well. The architectural note above is optional improvement, not a blocker. Good work tracking down the implicit-usings dependency.

Code Review

Overall: Clean, correct fix for a real bug. The root-cause diagnosis is accurate and the test strategy is well-thought-out.

Core Fix — TupleArgumentHelper.cs

The one-line change is correct. Extension methods require using System.Linq in scope; generated .g.cs files only have that when ImplicitUsings is enabled. Switching to fully-qualified static invocation is exactly right.

Architectural suggestion: Consider eliminating LINQ from generated output entirely rather than qualifying it. The pattern is a simple index-based cast — emitting a for-loop avoids any dependency on System.Linq in the output code:

// Rather than: Enumerable.ToArray(Enumerable.Select(Enumerable.Range(...)))
// emit a direct loop: new T[count] with indexed Cast<T> assignments

Benefits:

• Easier to read in generated output (important when debugging test failures)
• Removes the Enumerable dependency entirely rather than working around its surface syntax
• Avoids the IEnumerable intermediate allocation (minor, since ToArray follows immediately)

The current fix is correct for the issue at hand — this is a suggestion for a follow-up if generated-code ergonomics matter.

New Test — ParamsArrayCompilationTests.cs

This is exactly the right kind of regression test. It targets the blind spot identified in TestsBase: snapshot tests compile with injected global usings, so they cannot catch missing-using bugs in generated code. Compiling the generator output without System.Linq in scope correctly isolates this regression.

The approach is sound: output.GetDiagnostics() on the updated Compilation runs semantic analysis across all syntax trees including generated ones, so CS1061 surfaces here.

The captured generated variable in the failure message is a good diagnostics touch.

TestsBase.cs Comment

The warning explaining why snapshot tests won't catch this class of bug is a valuable addition. Future contributors will understand why ParamsArrayCompilationTests exists and why generated code must use fully-qualified calls.

Minor Notes

• new string[0] / new int[0] in generated fallback code could use Array.Empty<string>() / Array.Empty<int>() to avoid per-call allocations, but this is cosmetic.
• BOM removal in several snapshot files is a harmless bonus cleanup.

Verdict: Correct fix, well-scoped change, and the new compilation-level regression test addresses the specific gap in snapshot testing. The architectural note is optional follow-up, not a blocker.